Add pagination visitors stream #129
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 4 commits
November 8, 2023 10:31
Vi6hal
reviewed
Nov 9, 2023
tap_pendo/streams.py
Outdated
Comment on lines
739
to
742
| if self.name in ["visitors"]: | ||
| # If number of records equal to record then assume there are more records to be synced | ||
| # and save the last filter value. Otherwise assume we have extracted all records | ||
| loop_for_records = self.get_record_count() >= self.record_limit |
There was a problem hiding this comment.
I strongly suggest not following this approach as it creates inverted dependancy, this function belongs to the generic implementation of the stream class, and this condition refers to the name of a child class.
i would suggest that you modify this method in the Visitors class below
There was a problem hiding this comment.
or you can convert this into a generic implementation
|
I have added visitors test data, that will be available for testing tomorrow. Please expect change at base.py#L34 to reduce the daily execution time in CCi.
|
shantanu73
approved these changes
Nov 14, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of change
Added the the custom pagination support for the Visitors stream TDL-24201
Now visitor records will be synced in the batches of
record_limitwhich defaults to 100,000 instead of syncing them all in one go reducing the memory stress.Manual QA steps
include_anonymous_visitors="true"visitorsand child streamvisitor_historystreams are selectedRisks
Rollback steps